fix(parsing): reject malformed auth params#14
Conversation
|
hey think this has conflicts now that needs updating 🙇 |
|
i think its likely we need a conformance test added to mpp-tools also for this? |
Signed-off-by: EfeDurmaz16 <efebarandurmaz05@gmail.com>
EfeDurmaz16
force-pushed
the
fix/reject-malformed-auth-params
branch
from
2646972 to
335378c
Compare
|
Hey @emmajam I do see some failed tests that appear to overlap with this change (formatting, etc.) - is this change still relevant given some of the other improvements? |
|
|
||
| params_str.scan(AUTH_PARAM_RE) do |key, quoted_val, token_val| | ||
| match = T.must(Regexp.last_match) | ||
| separator = params_str[pos...match.begin(0)] |
There was a problem hiding this comment.
from_www_authenticate_list still splits on every /Payment\s+/, including inside quoted values. With the new strict tail validation, a header like Payment id="ch", realm="contact Payment support", ... is split at the quoted Payment, so the first chunk becomes malformed and parsing fails. The new mpp-tools vectors cover from_www_authenticate, but not this public list parser path.
| first = false | ||
| end | ||
|
|
||
| tail = params_str[pos..] |
There was a problem hiding this comment.
This now rejects the existing mpp-tools conformance vector unescaped_quotes_in_description, prob needs coordination w/ tempoxyz/mpp-tools#30 ?
| end | ||
|
|
||
| tail = params_str[pos..] | ||
| Kernel.raise Mpp::ParseError, "Malformed authentication parameters" unless tail.strip.empty? |
There was a problem hiding this comment.
this breaks the current mpp-tools vector; either update conformance or keep this case permissive.
5 participants
Summary
Reject malformed
WWW-Authenticate: Paymentauth-param lists instead of silently accepting partial regex matches.This catches cases where required params are present but the header is still malformed, for example:
Why
The parser previously used
scan, so malformed text between or after matched params could be ignored. For payment challenges, accepting a partially parsed header can hide producer bugs and make interop failures harder to diagnose.Verification
mise exec ruby@3.3 -- bundle exec ruby -Itest test/mpp/test_parsing.rbmise exec ruby@3.3 -- bundle exec rake testmise exec ruby@3.3 -- bundle exec standardrb lib/mpp/parsing.rb test/mpp/test_parsing.rbgit diff --check